Overhaul configuration#1948
Conversation
dask/dask#3432 centralizes configuration in dask/dask. This commit updates dask/distributed for these changes. Notably we centralize all default values in distributed/config.yaml which is now un-commented. We merge it into the global configuration by default on startup.
mrocklin
changed the title
|
This could use review by people familiar with configuration systems. cc @jakirkham @minrk @jacobtomlinson if you have time. |
jacobtomlinson
left a comment
jacobtomlinson
left a comment
There was a problem hiding this comment.
This looks like a nice overhaul. I've added a couple of comments but they are more questions that issues.
Looking at the test failures I imagine there are some outdated imports somewhere.
|
|
||
| def __init__(self, **kwargs): | ||
| self._init_from_dict(config) | ||
| self._init_from_dict(dask.config.config) |
There was a problem hiding this comment.
Did you mean to double .config here?
There was a problem hiding this comment.
Yes, the dask.config module has a config dict. See dask/dask#3432 for the co-PR
| user_priority = {k: user_priority for k in tasks} | ||
|
|
||
| priority = priority or order(tasks) # TODO: define order wrt old graph | ||
| priority = priority or dask.order.order(tasks) # TODO: define order wrt old graph |
There was a problem hiding this comment.
Did you mean to double .order here?
There was a problem hiding this comment.
Yes, same as above. This change is somewhat orthogonal to this PR, I just decided to start using namespaces more heavily rather than direct imports.
|
Tests will fail hard on this PR until dask/dask#3432 is merged. The next release will have a strict dependence between dask/dask and dask/distributed. |
dask/dask#3432 centralizes configuration in dask/dask. This commit updates dask/distributed for these changes. Notably we centralize all default values in distributed/config.yaml which is now un-commented. We merge it into the global configuration by default on startup.
* `logging` was removed from the config in 399522b (distributed PR dask#1948), and the assumption made in the present commit is that it was done unintentionally, since docs/source/configuration.rst (dask) wasn't updated at the same time with 399522b. * Later down the line, in response to https://stackoverflow.com/questions/57829474/dask-config-get-cannot-get-anything, (dask PR dask#5374) was merged, which didn't update the documentation either. * The above two facts, combined together, suggest that `logging` must've been removed unintentionally from the config.
2 participants
This is a large overhaul of our configuration. It depends on dask/dask#3432 which centralizes configuration code in dask/dask.
In particular this does the following:
{'scheduler-wait-time': ...}we get values more like{'scheduler': {'wait-time': ...}}.